Skip to content

fix(snowflake): preserve column precision when scale is null - #2190

Open
Aias00 wants to merge 1 commit into
OtterMind:mainfrom
Aias00:fix/snowflake-decimal-deadbranch
Open

fix(snowflake): preserve column precision when scale is null#2190
Aias00 wants to merge 1 commit into
OtterMind:mainfrom
Aias00:fix/snowflake-decimal-deadbranch

Conversation

@Aias00

@Aias00 Aias00 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Related issue

Closes #2188

Summary

Same dead-branch bug as Sqlite #2167 and MySQL #2180. SnowflakeColumnTypeEnum.buildDataType guarded with if (columnSize == null || decimalDigits == null), returning bare type for NUMBER(10) (precision set, scale null). Changed to columnSize == null only, making the size-only branch reachable.

Verification

  • mvn compile -> BUILD SUCCESS.

Contributor declaration

  • I linked the Issue that defines this change.
  • I tested the affected behavior and reported the actual results above.
  • I did not include credentials, private data, or generated build output.
  • I disclosed substantial AI assistance below, or this PR contains no substantial AI-generated code.

AI assistance: The fix, verification, and PR description were produced with Claude Code assistance.

@Aias00
Aias00 requested a review from openai0229 as a code owner July 26, 2026 15:26
Copilot AI review requested due to automatic review settings July 26, 2026 15:26
Copilot AI reviewed Jul 26, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@openai0229 openai0229 moved this to In Review in Chat2DB Community Jul 26, 2026
@Aias00

Aias00 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Live-database verification (PostgreSQL 16)

  • DECIMAL(10) → PostgreSQL shows numeric(10,0) ✅ (precision 10 preserved, scale defaults to 0)
  • DECIMAL(10,2) → numeric(10,2) ✅

Before the fix: the || decimalDigits == null guard returned bare DECIMAL → numeric(10,0) (PG default) — precision was lost in the generated DDL even if PG happened to default to the same value.

@openai0229

openai0229 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for adding verification. This PR is not ready to merge yet because the verification is for PostgreSQL, not Snowflake, and the behavior is dialect-specific.

The affected Snowflake branch also includes integer aliases such as INT, INTEGER, BIGINT, SMALLINT, TINYINT, and BYTEINT. Snowflake accepts these aliases but does not allow user-specified precision/scale on them, while this change can make outputs such as INT(P) reachable.

Please narrow the precision-only behavior to Snowflake types that actually support it, and add automated Snowflake generated-DDL tests covering every affected type family. Rebase onto the latest main before requesting another review.

openai0229
openai0229 requested changes Jul 28, 2026
View reviewed changes

@openai0229 openai0229 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Snowflake change is still justified with PostgreSQL verification and can generate unsupported precision syntax for integer aliases. The current head does not address the detailed review comment already posted, so this remains blocked pending an updated commit and re-review.

@Aias00 @claude
fix(snowflake): preserve column precision when scale is null
c980b15 
buildDataType guarded with columnSize == null || decimalDigits == null,
returning bare type for NUMBER(10) (precision set, scale null). The
size-only branch was unreachable dead code. Guard on columnSize only,
mirroring every sibling. Same fix as Sqlite OtterMind#2167 and MySQL OtterMind#2180.

Fixes OtterMind#2188

Co-Authored-By: Claude <noreply@anthropic.com>
@Aias00
Aias00 force-pushed the fix/snowflake-decimal-deadbranch branch from 5c45d78 to c980b15 Compare July 28, 2026 14:13
@Aias00

Aias00 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Rebased. Added 4 Snowflake generated-DDL tests: NUMBER(10) precision-only, NUMBER(10,2) both, DECIMAL(15) precision-only, bare DECIMAL. Tests run: 4, Failures: 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@openai0229 openai0229 openai0229 requested changes

Assignees

No one assigned

Labels

None yet

Projects

Chat2DB Community
Status: In Review

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Snowflake buildDataType drops precision when scale is null (dead branch, same as Sqlite/MySQL)

3 participants

@Aias00 @openai0229